Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Utils.waitUntilReady should record the stack trace of the caller before rethrowing an exception #1797

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

jglick
Copy link
Contributor

@jglick jglick commented Oct 4, 2019

I had a situation where one method in my code called a second, which called a third, which called Watchable.watch, and I think watch threw up an exception (in this case due to an RBAC mistake) which was caught ultimately in my first method (according to the method name recorded by java.util.logging)…but it was hard to tell for sure since the stack trace including watch was nowhere to be seen:

io.fabric8.kubernetes.client.KubernetesClientException: deployments.apps "etc" is forbidden: User "system:serviceaccount:somens:someacct" cannot watch resource "deployments" in API group "apps" in the namespace "somens"
	at io.fabric8.kubernetes.client.dsl.internal.WatchConnectionManager$1.onFailure(WatchConnectionManager.java:206)
	at okhttp3.internal.ws.RealWebSocket.failWebSocket(RealWebSocket.java:571)
	at okhttp3.internal.ws.RealWebSocket$2.onResponse(RealWebSocket.java:198)
	at okhttp3.RealCall$AsyncCall.execute(RealCall.java:206)
	at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

Poking around, my guess is that the problem is that Utils.waitUntilReady is taking this exception thrown and caught in some thread pool and rethrowing it to its caller. Somehow the caller’s stack trace needs to be included.

(Another idiom is to wrap the original exception in a new exception using cause, but that forces you to pick a type for the wrapper; using addSuppressed avoids that issue.)


I had hoped to test this change against the original reproduced problem; unfortunately I did not manage to compile this project, getting seemingly different puzzling errors from Maven or javac each time, like

Generating: …/fabric8io/kubernetes-client/kubernetes-model/kubernetes-model/target/classes/kubernetes.properties
Generating: …/fabric8io/kubernetes-client/kubernetes-model/kubernetes-model/target/classes/openshift.properties
An exception has occurred in the compiler (1.8.0_222). Please file a bug against the Java compiler via the Java bug reporting page (http://bugreport.java.com) after checking the Bug Database (http://bugs.java.com) for duplicates. Include your program and the following diagnostic in your report. Thank you.
java.lang.IllegalStateException: endPosTable already set
	at com.sun.tools.javac.util.DiagnosticSource.setEndPosTable(DiagnosticSource.java:136)
	at com.sun.tools.javac.util.Log.setEndPosTable(Log.java:350)
	at com.sun.tools.javac.main.JavaCompiler.parse(JavaCompiler.java:667)
	at com.sun.tools.javac.main.JavaCompiler.parseFiles(JavaCompiler.java:950)
	at com.sun.tools.javac.processing.JavacProcessingEnvironment$Round.<init>(JavacProcessingEnvironment.java:900)
	at com.sun.tools.javac.processing.JavacProcessingEnvironment$Round.next(JavacProcessingEnvironment.java:929)
	at com.sun.tools.javac.processing.JavacProcessingEnvironment.doProcessing(JavacProcessingEnvironment.java:1195)
	at com.sun.tools.javac.main.JavaCompiler.processAnnotations(JavaCompiler.java:1170)
	at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:856)
	at com.sun.tools.javac.main.Main.compile(Main.java:523)
	at com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:129)
	at com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:138)
	at org.codehaus.plexus.compiler.javac.JavaxToolsCompiler.compileInProcess(JavaxToolsCompiler.java:126)
	at org.codehaus.plexus.compiler.javac.JavacCompiler.performCompile(JavacCompiler.java:174)
	at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:1134)
	at org.apache.maven.plugin.compiler.CompilerMojo.execute(CompilerMojo.java:187)
	at …

or

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project kubernetes-model: Compilation failure: Compilation failure: 
[ERROR] …/fabric8io/kubernetes-client/kubernetes-model/kubernetes-model/src/main/java/io/fabric8/openshift/api/model/Template.java:[105,18] cannot find symbol
[ERROR]   symbol:   class Parameter
[ERROR]   location: class io.fabric8.openshift.api.model.Template

or

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project kubernetes-client: Fatal error compiling: java.lang.RuntimeException: java.io.FileNotFoundException: resource-handler-services.vm -> [Help 1]

@centos-ci
Copy link

Can one of the admins verify this patch?

@rohanKanojia
Copy link
Member

ok to test

@rohanKanojia rohanKanojia requested review from iocanel and oscerd October 7, 2019 08:42
@rohanKanojia rohanKanojia added the changelog missing A line to changelog.md regarding the change is not added label Oct 7, 2019
@@ -130,7 +130,9 @@ public static boolean waitUntilReady(BlockingQueue<Object> queue, long amount, T
if (obj instanceof Boolean) {
return (Boolean) obj;
} else if (obj instanceof Throwable) {
throw (Throwable) obj;
Throwable t = (Throwable) obj;
t.addSuppressed(new Throwable("waiting here"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we be more expressive in the message inside throwable? Or does this message has not any significance?

Copy link
Member

@rohanKanojia rohanKanojia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor question, Looks good to me 👍 .

@rohanKanojia rohanKanojia removed the changelog missing A line to changelog.md regarding the change is not added label Oct 9, 2019
@rohanKanojia
Copy link
Member

[merge]

@fusesource-ci fusesource-ci merged commit a572527 into fabric8io:master Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants